Skip to content

chore: Fixing renaming of Queries, JS and Widgets to replace spaces and special characters with underscore#39596

Merged
ankitakinger merged 2 commits intoreleasefrom
chore/remove-old-component
Mar 6, 2025
Merged

chore: Fixing renaming of Queries, JS and Widgets to replace spaces and special characters with underscore#39596
ankitakinger merged 2 commits intoreleasefrom
chore/remove-old-component

Conversation

@ankitakinger
Copy link
Contributor

@ankitakinger ankitakinger commented Mar 6, 2025 •
edited by github-actions bot
Loading

Description

  • Fixing renaming of Queries, JS and Widgets to replace spaces and special characters with underscore
  • Also fixes the issue where UI breaks during renaming when trying to rename with longer names
  • Removing duplicate component which is now moved to ADS

Fixes #39589 #39598 #39599

Automation

/ok-to-test tags="@tag.IDE"

🔍 Cypress test results

Tip

🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
Workflow run: https://github.com/appsmithorg/appsmith/actions/runs/13698453955
Commit: 523b970
Cypress dashboard.
Tags: @tag.IDE
Spec:

Thu, 06 Mar 2025 12:48:08 UTC

Communication

Should the DevRel and Marketing teams inform users about this change?

  • Yes
  • No

Summary by CodeRabbit

  • New Features
    • Updated a context menu component to require menu content, ensuring users always supply the necessary menu items.
  • Chores
    • Removed a duplicate version of the context menu component to streamline the interface.
  • Bug Fixes
    • Changed the default behavior of the normalizeName property in the EditableEntityName component to apply normalization by default.
    • Updated the default behavior of the normalizeName property in the EntityItem component to normalize names by default.

@ankitakinger ankitakinger requested a review from a team as a code owner March 6, 2025 12:03
@ankitakinger ankitakinger added the ok-to-test Required label for CI label Mar 6, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 6, 2025 •
edited
Loading

Walkthrough

This pull request enforces a required children prop in the design system's EntityContextMenu component by updating its TypeScript interface. Additionally, it removes the legacy IDE-specific EntityContextMenu implementation, thereby consolidating the component usage within the design system. The default values for the normalizeName property in both the EditableEntityName and EntityItem components are also updated to true.

Changes

File Change Summary
app/client/packages/design-system/ads/src/Templates/EntityContextMenu/EntityContextMenu.tsx Updated the Props interface by changing the children property from optional to required.
app/client/src/IDE/Components/EntityContextMenu.tsx Deleted the legacy IDE-specific EntityContextMenu component.
app/client/packages/design-system/ads/src/Templates/EditableEntityName/EditableEntityName.tsx Changed the default value of normalizeName from false to true.
app/client/packages/design-system/ads/src/Templates/EntityExplorer/EntityItem/EntityItem.tsx Changed the default value of normalizeName in nameEditorConfig from false to true.

Possibly related PRs

Suggested labels

Task, IDE Product, IDE Pod

Suggested reviewers

  • ApekshaBhosale

Poem

In the code we now refine,
Children must in structure shine.
Old files gently fade away,
Making way for a brighter day.
A cleaner code with focus bright,
Guiding us through day and night.
Cheers to progress and code delight!

📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro (Legacy)

📥 Commits

Reviewing files that changed from the base of the PR and between eb5815d and 523b970.

📒 Files selected for processing (2)
  • app/client/packages/design-system/ads/src/Templates/EditableEntityName/EditableEntityName.tsx (1 hunks)
  • app/client/packages/design-system/ads/src/Templates/EntityExplorer/EntityItem/EntityItem.tsx (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (10)
  • GitHub Check: perform-test / rts-build / build
  • GitHub Check: perform-test / server-build / server-unit-tests
  • GitHub Check: perform-test / client-build / client-build
  • GitHub Check: client-unit-tests / client-unit-tests
  • GitHub Check: client-prettier / prettier-check
  • GitHub Check: client-lint / client-lint
  • GitHub Check: client-build / client-build
  • GitHub Check: chromatic-deployment
  • GitHub Check: chromatic-deployment
  • GitHub Check: storybook-tests
🔇 Additional comments (2)
app/client/packages/design-system/ads/src/Templates/EntityExplorer/EntityItem/EntityItem.tsx (1)

20-20: Default value for normalizeName is now true

This change aligns the default behavior with the EditableEntityName component, ensuring consistent name normalization across the design system.

app/client/packages/design-system/ads/src/Templates/EditableEntityName/EditableEntityName.tsx (1)

25-25:

Details

❓ Verification inconclusive

Default value for normalizeName is now true

This change establishes consistency in how entity names are handled throughout the application. The normalization will now be applied by default, which is a behavioral change that might affect the display of entity names.

🏁 Script executed:

#!/bin/bash
# Check if there are any tests that explicitly set normalizeName to false
# to ensure they won't be affected by this change

rg -A 2 -B 2 "normalizeName={false}" --glob="*.{tsx,jsx,ts,js}"

Length of output: 184

Entity Name Normalization Consistency Check

The change to default normalizeName = true is consistent with our intention to standardize entity name handling. However, please verify that no tests or components are still relying on an explicit normalizeName={false}. To aid in this, you can re-run the search using a corrected regex. For example:

If this revised command returns no results, it confirms that no tests are adversely affected by the new default. Please perform this manual check before finalizing the merge.

🏁 Script executed:

#!/bin/bash
rg -A 2 -B 2 'normalizeName=\{false\}' --glob="*.{tsx,jsx,ts,js}"

Length of output: 189

Entity Name Normalization Default Value Verification

The change setting normalizeName = true ensures consistent handling of entity names. Our automated search (with the adjusted regex) did not reveal any instances of normalizeName={false}, suggesting no tests or components explicitly override this behavior. However, due to the minor issues seen with the shell scripted command, please manually verify that no dependencies rely on a false value.

  • Location: app/client/packages/design-system/ads/src/Templates/EditableEntityName/EditableEntityName.tsx at line 25
  • Action: Manually check for any usage of normalizeName={false} in tests/components to ensure they are not inadvertently affected.
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@ankitakinger ankitakinger requested a review from hetunandu March 6, 2025 12:03
@github-actions github-actions bot added the skip-changelog Adding this label to a PR prevents it from being listed in the changelog label Mar 6, 2025
hetunandu
hetunandu previously approved these changes Mar 6, 2025
View reviewed changes
@ankitakinger ankitakinger changed the title chore: Removing duplicate component which is now moved to ADS chore: Fixing renaming of Queries and JS to replace spaces and special characters with underscore Mar 6, 2025
@github-actions github-actions bot added Bug Something isn't working IDE Pod Issues that new developers face while exploring the IDE IDE Product Issues related to the IDE Product Medium Issues that frustrate users due to poor UX Needs Triaging Needs attention from maintainers to triage Release labels Mar 6, 2025
@ankitakinger ankitakinger requested a review from hetunandu March 6, 2025 12:17
@github-actions github-actions bot added Bug Something isn't working and removed Bug Something isn't working labels Mar 6, 2025
@ankitakinger ankitakinger changed the title chore: Fixing renaming of Queries and JS to replace spaces and special characters with underscore chore: Fixing renaming of Queries, JS and Widgets to replace spaces and special characters with underscore Mar 6, 2025
@github-actions github-actions bot added Bug Something isn't working and removed Bug Something isn't working labels Mar 6, 2025
@ankitakinger
Copy link
Contributor Author

ankitakinger commented Mar 6, 2025

/build-deploy-preview skip-tests=true

@github-actions
Copy link

github-actions bot commented Mar 6, 2025

Deploying Your Preview: https://github.com/appsmithorg/appsmith/actions/runs/13698675896.
Workflow: On demand build Docker image and deploy preview.
skip-tests: true.
env: ``.
PR: 39596.
recreate: .

@github-actions
Copy link

github-actions bot commented Mar 6, 2025

Deploy-Preview-URL: https://ce-39596.dp.appsmith.com

@ankitakinger ankitakinger merged commit 0d1f4f7 into release Mar 6, 2025
62 checks passed
@ankitakinger ankitakinger deleted the chore/remove-old-component branch March 6, 2025 12:54
@coderabbitai coderabbitai bot mentioned this pull request Mar 7, 2025
Merged
2 tasks
github-actions bot pushed a commit to Zeral-Zhang/appsmith that referenced this pull request Apr 12, 2025
@ankitakinger
chore: Fixing renaming of Queries, JS and Widgets to replace spaces a…
103766d 
…nd special characters with underscore (appsmithorg#39596)

## Description

- Fixing renaming of Queries, JS and Widgets to replace spaces and
special characters with underscore
- Also fixes the issue where UI breaks during renaming when trying to
rename with longer names
- Removing duplicate component which is now moved to ADS

Fixes [appsmithorg#39589](appsmithorg#39589)
[appsmithorg#39598](appsmithorg#39598)
[appsmithorg#39599](appsmithorg#39599)

## Automation

/ok-to-test tags="@tag.IDE"

### 🔍 Cypress test results
<!-- This is an auto-generated comment: Cypress test results  -->
> [!TIP]
> 🟢 🟢 🟢 All cypress tests have passed! 🎉 🎉 🎉
> Workflow run:
<https://github.com/appsmithorg/appsmith/actions/runs/13698453955>
> Commit: 523b970
> <a
href="https://internal.appsmith.com/app/cypress-dashboard/rundetails-65890b3c81d7400d08fa9ee5?branch=master&workflowId=13698453955&attempt=1"
target="_blank">Cypress dashboard</a>.
> Tags: `@tag.IDE`
> Spec:
> <hr>Thu, 06 Mar 2025 12:48:08 UTC
<!-- end of auto-generated comment: Cypress test results  -->


## Communication
Should the DevRel and Marketing teams inform users about this change?
- [ ] Yes
- [ ] No


<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit

- **New Features**
- Updated a context menu component to require menu content, ensuring
users always supply the necessary menu items.
- **Chores**
- Removed a duplicate version of the context menu component to
streamline the interface.
- **Bug Fixes**
- Changed the default behavior of the `normalizeName` property in the
`EditableEntityName` component to apply normalization by default.
- Updated the default behavior of the `normalizeName` property in the
`EntityItem` component to normalize names by default.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@hetunandu hetunandu hetunandu approved these changes

Assignees

No one assigned

Labels

IDE Pod Issues that new developers face while exploring the IDE IDE Product Issues related to the IDE Product Medium Issues that frustrate users due to poor UX Needs Triaging Needs attention from maintainers to triage ok-to-test Required label for CI Release skip-changelog Adding this label to a PR prevents it from being listed in the changelog

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[Bug]: Entity Explorer with ADS - Issues with renaming queries/JS Objects

2 participants

@ankitakinger @hetunandu